Skip to content

refactor(api): remove test-only model re-export from workspaces#118

Merged
wpak-ai merged 2 commits into
masterfrom
refactor/remove-test-only-reexport
Jun 24, 2026
Merged

refactor(api): remove test-only model re-export from workspaces#118
wpak-ai merged 2 commits into
masterfrom
refactor/remove-test-only-reexport

Conversation

@clean6378-max-it

@clean6378-max-it clean6378-max-it commented Jun 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

closes #109

Summary

Remove the test-only from models import Bubble, Composer, Workspace # noqa: F401 re-export from api/workspaces.py. Refactor tests/test_models_wired_at_read_sites.py to patch model classes at their actual usage sites in services/ instead of relying on an API-layer re-export.

Sprint item #6 (Tuesday PR 2 of 2, 3 pt).

Problem

api/workspaces.py contained imports that existed only to support the wired-at-read-sites spy harness. That let test implementation details shape the production import surface.

Changes

  • api/workspaces.py: remove the test-only model re-export block and comment; remove unused lookup_workspace_display_name import
  • tests/test_models_wired_at_read_sites.py: retarget spy patches to real consumption sites:
    • services.workspace_db.Bubble for /api/workspaces/.../tabs bubble validation
    • services.workspace_tabs.Composer for /api/workspaces/.../tabs composer validation
    • services.workspace_resolver.Workspace for lookup_workspace_display_name

Out of scope

Test plan

  • ruff check api/workspaces.py
  • pytest tests/test_models_wired_at_read_sites.py (13 passed)
  • pytest (full suite)
  • mypy --strict (if run in CI)

Acceptance criteria

  • Re-export import and comment removed from api/workspaces.py
  • Spy tests patch model classes at actual services/ usage sites
  • Same invariant verified without API-layer re-exports
  • api/workspaces.py imports only symbols it uses (ruff F401 clean)

Summary by CodeRabbit

  • Refactor
    • Streamlined internal imports by removing unused model exports and relying on the workspace name resolver directly.
  • Tests
    • Updated regression tests to patch the model deserialization points actually used by each production read path, improving spy correctness.
    • Cleaned up an unnecessary logging import in a schema drift test.

@coderabbitai

coderabbitai Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cb046634-95a7-454b-8a25-184b334b6e63

📥 Commits

Reviewing files that changed from the base of the PR and between 63cdfcb and 083c539.

📒 Files selected for processing (1)
  • tests/test_models_wired_at_read_sites.py
💤 Files with no reviewable changes (1)
  • tests/test_models_wired_at_read_sites.py
📝 Walkthrough

Walkthrough

api/workspaces.py removes model re-exports (Bubble, Composer, Workspace) that existed solely for test spies and drops the unused lookup_workspace_display_name import. Three corresponding tests in test_models_wired_at_read_sites.py are updated to patch those models at their actual service-layer locations (services.workspace_db, services.workspace_tabs, services.workspace_resolver), and one redundant import logging statement is removed.

Changes

Test spy re-import cleanup

Layer / File(s) Summary
Remove test-only re-exports and unused import
api/workspaces.py		 Drops the Bubble, Composer, Workspace re-export block and removes lookup_workspace_display_name from the workspace_resolver import, leaving only infer_workspace_name_from_context.
Update spy patch targets to actual service-layer sites
tests/test_models_wired_at_read_sites.py		 Patches Bubble.from_dict at services.workspace_db, Composer.from_dict at services.workspace_tabs, and Workspace.from_dict at services.workspace_resolver; imports within each test are adjusted to match the new patch paths, and a redundant import logging statement is removed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Possibly related PRs

  • cppalliance/cppa-cursor-browser#30: The main PR's cleanup removes api/workspaces.py re-exports of Bubble/Composer/Workspace and adjusts tests to patch the actual *.from_dict call sites, which directly relates to the retrieved PR's typed-model (Bubble/Composer/Workspace) schema-validation wiring at those read paths.

Suggested reviewers

  • bradjin8
  • timon0305
  • wpak-ai

Poem

🐇 Hop, hop, the re-exports are gone!
No sneaky re-imports to carry on.
The spies now watch the services true,
workspace_db, workspace_tabs, and resolver too.
A cleaner burrow for every new dawn! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(api): remove test-only model re-export from workspaces' clearly and concisely summarizes the main change: removing the test-only re-export block from api/workspaces.py.
Linked Issues check ✅ Passed The PR fully satisfies all coding-related acceptance criteria from #109: re-export removed, tests refactored to patch at actual usage sites (services.workspace_db, services.workspace_tabs, services.workspace_resolver), unused import eliminated, and invariant preserved.
Out of Scope Changes check ✅ Passed All changes directly align with the #109 scope: only the re-export block removal from api/workspaces.py and test refactoring to patch at actual usage sites in services/ are included; no unrelated modifications present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/remove-test-only-reexport

Comment @coderabbitai help to get the list of available commands.

@clean6378-max-it clean6378-max-it self-assigned this Jun 24, 2026
@timon0305 timon0305 mentioned this pull request Jun 24, 2026
Merged
9 tasks
clean6378-max-it added a commit that referenced this pull request Jun 24, 2026
@clean6378-max-it
revert(workspaces): restore lookup_workspace_display_name import for #…
83ae445 
…118
@clean6378-max-it clean6378-max-it requested a review from wpak-ai June 24, 2026 12:33
wpak-ai pushed a commit that referenced this pull request Jun 24, 2026
@clean6378-max-it
docs: add Google-style docstrings to public api/services/utils/models… (
c97a37c 
#119)

* docs: add Google-style docstrings to public api/services/utils/models surface

* docs: correct inaccurate API and model docstrings from PR review

* docs: address PR 119 review — Google-style sections and doc accuracy

* docs: complete validate_path and get_workspace_tab docstrings

* revert(workspaces): restore lookup_workspace_display_name import for #118
@wpak-ai wpak-ai merged commit 2f728d8 into master Jun 24, 2026
16 checks passed
@wpak-ai wpak-ai deleted the refactor/remove-test-only-reexport branch June 24, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wpak-ai wpak-ai wpak-ai approved these changes

@timon0305 timon0305 timon0305 approved these changes

Assignees

@clean6378-max-it clean6378-max-it

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cppa-cursor-browser: Test spy re-import cleanup in api/workspaces.py

3 participants

@clean6378-max-it @wpak-ai @timon0305